Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added client keepalive arguments to the grpc-proxy #17366

Merged
merged 4 commits into from
Feb 19, 2024

Conversation

halleyshx
Copy link
Contributor

Added dial-keepalive-time、dial-keepalive-timeout、permit-without-stream arguments to the grpc-proxy for KeepAlive ClientParameters.

Currently cluster's request link: clients(A、B、C) --> grpc-proxy(start by c1.etcd.com) --> etcd-cluster-domain(c1.etcd.com) --> loadbalancer(11.11.11.11) --> etcd endpoints(e1、e2、e3).

When loadbalancer(11.11.11.11) died, new loadbalancer(11.11.11.12) to replaced, grpc-proxy can't provide service to his clients(A、B、C) for 15min.(why 15min?because gRPC balancergroup's balancerCache is default 15min in gPRC-1.38.0 var DefaultSubBalancerCloseTimeout = 15 * time.Minute ), it's too late.

So we need to actively probe for loadbalancer of etcd-cluster-domain survival and reconnect.
Currently added client keepalive arguments to the grpc-proxy.

This PR introduces three options dial-keepalive-time、dial-keepalive-timeout、permit-without-stream for grpc-proxy:
dial-keepalive-time allows to specify client keepalive interval for grpc-proxy itself (default 0-disable).
dial-keepalive-timeout allows to specify client keepalive timeout for grpc-proxy itself (default 20s).
permit-without-stream allows to specify client sends keepalive pings even with no active RPCs for grpc-proxy itself (default false).

When I start grpc-proxy with --dial-keepalive-time '20s', It worked.

…m arguments to the grpc-proxy

Signed-off-by: shihuixing <shihuixing@jd.com>
@k8s-ci-robot
Copy link

Hi @halleyshx. Thanks for your PR.

I'm waiting for a etcd-io member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@halleyshx
Copy link
Contributor Author

please recheck,thanks @ahrtr

@ahrtr
Copy link
Member

ahrtr commented Feb 5, 2024

/ok-to-test

server/etcdmain/grpc_proxy.go Outdated Show resolved Hide resolved
Signed-off-by: shihuixing <shihuixing@jd.com>
server/etcdmain/grpc_proxy.go Outdated Show resolved Hide resolved
@ahrtr
Copy link
Member

ahrtr commented Feb 8, 2024

If you backport the PR to 3.5 (I have no objection on it), then add a changelog for 3.5 after you raise the backporting PR. thx

Signed-off-by: shihuixing <shihuixing@jd.com>
@halleyshx
Copy link
Contributor Author

If you backport the PR to 3.5 (I have no objection on it), then add a changelog for 3.5 after you raise the backporting PR. thx

OK,I updated the changelog-3.5. Please recheck. thx

@ahrtr
Copy link
Member

ahrtr commented Feb 9, 2024

OK,I updated the changelog-3.5. Please recheck. thx

PLease read my previous comment . Pls do not update changelog in this PR. Instead, update the changelog after raising the backporting (to 3.5) PR

Signed-off-by: shihuixing <shihuixing@jd.com>
@halleyshx
Copy link
Contributor Author

OK,I updated the changelog-3.5. Please recheck. thx

PLease read my previous comment . Pls do not update changelog in this PR. Instead, update the changelog after raising the backporting (to 3.5) PR

Ok. removed. But How should I raise the backporting (to 3.5) PR?

@ahrtr
Copy link
Member

ahrtr commented Feb 13, 2024

But How should I raise the backporting (to 3.5) PR?

Make the same change for https://github.com/etcd-io/etcd/tree/release-3.5.

@halleyshx
Copy link
Contributor Author

But How should I raise the backporting (to 3.5) PR?

Make the same change for https://github.com/etcd-io/etcd/tree/release-3.5.

Happy new Year!
Please check #17444 to release-3.5. But where is changelog rewritten, I didn't find it in release-3.5.

@ahrtr
Copy link
Member

ahrtr commented Feb 18, 2024

Please check #17444 to release-3.5. But where is changelog rewritten, I didn't find it in release-3.5.

Update https://github.com/etcd-io/etcd/blob/main/CHANGELOG/CHANGELOG-3.5.md#v3513-tbd

@ahrtr
Copy link
Member

ahrtr commented Feb 18, 2024

Happy new Year!

thx. Happy new year!

@halleyshx
Copy link
Contributor Author

Please check #17444 to release-3.5. But where is changelog rewritten, I didn't find it in release-3.5.

Update https://github.com/etcd-io/etcd/blob/main/CHANGELOG/CHANGELOG-3.5.md#v3513-tbd

Sorry! Please check #17447.
And Update https://github.com/etcd-io/etcd/blob/main/CHANGELOG/CHANGELOG-3.5.md#v3513-tbd by this PR? or make a new PR to only update the changelog in CHANGELOG-3.5.md#v3513-tbd?

@ahrtr
Copy link
Member

ahrtr commented Feb 18, 2024

make a new PR to only update the changelog in CHANGELOG-3.5.md#v3513-tbd?

Pls create a new PR.

@halleyshx
Copy link
Contributor Author

make a new PR to only update the changelog in CHANGELOG-3.5.md#v3513-tbd?

Pls create a new PR.

OK, Pls check #17448 . thx!

@ahrtr ahrtr merged commit 97fc1ec into etcd-io:main Feb 19, 2024
39 checks passed
@halleyshx
Copy link
Contributor Author

make a new PR to only update the changelog in CHANGELOG-3.5.md#v3513-tbd?

Pls create a new PR.

OK, Pls check #17448 . thx!

update changelog by #17488

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants